Skip to content

fix: honor branch anchor destination on split-to-merge flows#311

Open
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:audit/writer-preserve-anchor-from-to-pairs
Open

fix: honor branch anchor destination on split-to-merge flows#311
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:audit/writer-preserve-anchor-from-to-pairs

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 26, 2026

Part of #332.

Fixes #310.

Summary

IF branch anchors on direct split-to-merge flows preserved the branch origin side but dropped the branch destination side.

Root cause

Several split-to-merge shortcuts called applyUserAnchors with the branch anchor as the origin override and nil as the destination override. That let the default destination side replace the user-provided branch to side.

Fix

Use the branch anchor for both origin and destination overrides in the split-to-merge shortcut paths, matching the existing behavior for branches with a first statement.

Tests

Added focused builder coverage for branch destination anchors on split-to-merge flows.

Validation

  • make build
  • make test
  • make lint-go
  • make test-integration

Agentic Code Testing

  • No new authoring syntax.
  • Synthetic builder regression coverage verifies anchor metadata survives generated MDL roundtrips.

Test plan

  • False/true branch destination sides survive a builder roundtrip.
  • Existing anchor defaults remain covered by the full test suite.

Branch-level `@anchor(... true: (...), false: (...))` metadata must preserve both the origin and destination sides for direct split-to-merge shortcut flows.

The builder applied only the branch origin side in several shortcut paths. With no destination override, the flow fell back to the default destination side and the next describe changed the branch anchor.

Pass the same branch anchor as both origin and destination override for split-to-merge shortcuts, matching the existing behavior for branches that have a first statement.

Tests cover true and false branch destination sides surviving a builder roundtrip.
@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

None found.

What Looks Good

  • Bug fix accuracy: The PR correctly identifies and fixes the root cause where split-to-merge shortcuts were dropping the branch destination side by passing nil as the destination override to applyUserAnchors. The fix uses the branch anchor for both origin and destination overrides, matching existing behavior.
  • Test coverage: Added two focused regression tests (TestBuilder_AnchorFalseBranchTo_IfWithoutElse and TestBuilder_AnchorTrueBranchTo_EmptyThenIfWithElse) that specifically verify branch destination anchors survive builder roundtrips for split-to-merge flows.
  • Code consistency: Changes are applied uniformly across all four split-to-merge shortcut paths in addIfStatement, maintaining parallel structure with explanatory comments.
  • Atomic scope: PR addresses a single issue (branch anchor destination preservation) without unrelated changes.
  • Validation: All standard validation checks pass (make build, make test, make lint-go, make test-integration).

Recommendation

Approve the PR. The fix is correct, well-tested, and follows project conventions. No changes are needed.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

Adds an MDL script under mdl-examples/bug-tests/ exercising IF-without-else
and IF-with-empty-then patterns where the branch anchor `to:` side must
survive a describe → exec → describe cycle.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

  • None found

Moderate Issues

  • None found

Minor Issues

  • None found

What Looks Good

  • The PR correctly addresses the bug by ensuring branch anchor destination is honored in split-to-merge flows
  • Test coverage is excellent:
    • New bug test file in mdl-examples/bug-tests/ that reproduces the issue and documents expected behavior
    • Comprehensive unit tests added covering both true and false branch scenarios in various IF statement configurations
    • Tests verify anchor metadata survives roundtrips (describe → exec → describe)
  • Fix is minimal and targeted: only three call sites in cmd_microflows_builder_control.go were modified to pass the branch anchor for both origin and destination overrides
  • Changes align with existing behavior for branches with first statements
  • No MDL syntax changes were needed (pure backend fix)
  • All validation checks pass (build, test, lint, integration)
  • Agentic testing confirms no new authoring syntax and verifies regression coverage
  • PR is scoped to a single concern (bug fix Split-to-merge flows lose branch destination anchors #310) with atomic commits

Recommendation

Approve - The PR is ready for merging. It correctly fixes the reported issue with appropriate test coverage and follows all project guidelines. No changes are needed.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split-to-merge flows lose branch destination anchors

2 participants